Adds an autocorrect to RSpec/ExpectActual, swapping the arguments to expect and matcher when possible.
#858
Conversation
| @@ -41,11 +41,33 @@ class ExpectActual < Cop | |||
| regexp | |||
| ].freeze | |||
|
|
|||
| def_node_matcher :expect_literal, '(send _ :expect $#literal?)' | |||
| SUPPORTED_MATCHERS = %i[eq eql equal be].freeze | |||
There was a problem hiding this comment.
it might be nice to have the option to add custom matchers in the configuration. This is another feature requests though
| actual, matcher, expected = expect_literal(node) | ||
| lambda do |corrector| | ||
| return unless SUPPORTED_MATCHERS.include?(matcher) | ||
| return if expected.is_a?(Array) # `be` without args |
There was a problem hiding this comment.
The comment is not precise, as arguments will also result in having an array. The :== case does not result in an array because of the $_ (while $... results in an array). So you are not correcting be, unless followed by ==
There was a problem hiding this comment.
Nice catch, the be with an argument case completely slipped my mind 👍
I assumed I'm using $... as a placeholder just to balance the number of captures in patter union clause, and it will always be [] for be.
pirj
force-pushed
the
expect_actual-autocorrect
branch
2 times, most recently
from
3e94f0d
to
701ccfe
Compare
| describe Foo do | ||
| it 'uses expect incorrectly' do | ||
| expect(1).to be | ||
| ^ Provide the actual you are testing to `expect(...)`. |
There was a problem hiding this comment.
Well, we don't have an actual here, and supporting this case complicates the pattern and the autocorrect code
pirj
force-pushed
the
expect_actual-autocorrect
branch
from
7349a8a
to
99be516
Compare
There was a problem hiding this comment.
The autocorrect might not be safe though. Maybe we can add SafeAutocorrect: false to the config, although we didn't provide the full meta data yet (but Safe and SafeAutocorrect are used by Rubocop, so at least it won't run this autocorrection in a Safe context)
| #{Runners::ALL.node_pattern_union} | ||
| { | ||
| (send (send nil? $:be) :== $_) | ||
| (send nil? $:be $_) |
There was a problem hiding this comment.
I think the matcher below handles this case as well, as be is in the supported matchers
pirj
force-pushed
the
expect_actual-autocorrect
branch
from
219f585
to
f152aaa
Compare
|
Thanks, guys! |
Based on #855 by @dduugg
Before submitting the PR make sure the following are checked:
master(if not - rebase it).bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).